-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rustdoc: use a button instead of a bar for search #133279
base: master
Are you sure you want to change the base?
Conversation
This is a response to complaints that the header area takes up too much vertical space, forcing the user to scroll more than they ought to need. It also adds a reasonable way to pick the crate name before actually searching, which was also a feature that people ask for.
This comment has been minimized.
This comment has been minimized.
I'm not too sure what to think about it. Turning the search input into a button (to make the search input appear) makes it more tricky to access the search feature. Also, on mobile it's really not great as the space available is very little. However I like a few things from this PR:
|
This comment has been minimized.
This comment has been minimized.
making things behave like links instead of magically doing stuff on hover is much better ux, that's the main reason I abandoned the old popover-on-hover PR. |
The whole mobile experience is big downgrade: the search button is small compared to the rest of the elements, and it's not necessarily obvious what we'll be search. The search input is doing a perfect job for this case. For desktop, like I said I'm very shared. I like some improvements, but having to click on a button to have access to the search feature is (in my opinion) a big inconvenience. The search in the rust docs is a major feature and something we want to make as accessible as possible. Hiding it behind a button, even if it seems like a small inconvenience is actually (again, since in my opinion) a huge step back.
Fair enough. Maybe we could have some "down arrows" button appearing below the search input when focused which would switch to this view? |
I think anything conditional on focus is an anti-pattern (yes, I'm aware I was the one who suggested that idea in the first place, but after I implemented it, I realized just how annoying it was). Anyone trying to navigate via keyboard input will naturally want to focus that button before selecting it, which will be extremely confusing if it is only visible when a certain other element is focused. How's this for an interesting compromise: a button that looks like a search bar |
Then what's the point into having a button and not just the input? |
the point is to have a single source of truth for "are we in search mode", and to allow other elements to only show up in search mode. |
Yes but when convenience is sacrificed in exchange of new context, it's not really an improvement. |
What convenience is sacrificed? clicking or tapping on the search button would immediately activate search mode, just like before. |
Except on mobile you need to go back to the previous history page to exit it. Also it adds an extra complexity in JS to handle this button we didn't need before. |
trivially easy on andriod, i guess we could add an "exit search mode" button on mobile for the benefit of iOS users. this is also only really a problem if you frequently start a search and then decide you actually don't want to do a search after all.
the bulk of the extra "complexity" is actually just the logic for building the example queries (which is a feature that I was planning on implementing if noone else did) otherwise it's pretty close to neutral, mostly just stuff being moved around and lazily initialized. |
This is not trivially easy. It completely breaks the browsing experience.
It remains new code added for benefits I'm still not sure to see. |
Continuing the mobile concept of moving the settings and help buttons up into the top bar, the search and summary might have room for real labels? The third screenshot is current nightly, meant to show how, even though we're line wrapping more, we're still coming out ahead for vertical screen real estate taken up in total.
How is that different from the current setup? The bar says "Press `s` or `/` to search, `?` for more options..." — what part of that phrase says anything about the scope of the search?
That would be a mystery meat icon, because its meaning is potentially ambiguous. Clicking on a down arrow next so a search field might do many different things:
I've taken every opportunity I could find (without significant regressions anywhere else) to reduce and avoid icons without labels, for exactly that reason. They're never as clear as you think they are.
I can add an explicit return to docs button for mobile users. It's already there, but I didn't think to label it. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
r? @GuillaumeGomez
CC @lolbinarycat @liigo
This is an attempt to address three complaints:
The header area takes up a lot of vertical space, and is generally quite "heavy" feeling.
Fixes [rustdoc] issues of the three-big-buttons #132386
Back and forward are kind of wacky, because there's no "single source of truth" for whether the search mode is open or not. Can't find an issue, but here's a screen recording where I reproduce it.
screen-record-history-bug.mp4
https://imgur.com/ZVnHv3o
You can't see the crate picker list, or anything else that we might want to show you, until after you've already typed your search term. [rustdoc search] allow setting crates to search in before showing results #129537 (also notice the example searches, which exist in Hoogle and this tweak adds)
Preview page: https://notriddle.com/rustdoc-html-demo-15/search-button/std/index.html
Video:
screen-record.mp4
https://imgur.com/c37VxLb
Screenshot: